*: report read statistics to pd #2307
Conversation
src/coprocessor/endpoint.rs
Outdated
|
|
||
| impl<R> ContextFactory<CopContext<R>> for CopContextFactory<R> | ||
| where | ||
| R: RaftStoreRouter + 'static, |
There was a problem hiding this comment.
IMO, we should not introduce RaftStoreRouter to the coprocessor, seem strange here.
You can define a trait which sending your read stats like storage raftkv does.
src/raftstore/store/msg.rs
Outdated
| @@ -75,6 +76,8 @@ pub enum Msg { | |||
| // For snapshot stats. | |||
| SnapshotStats, | |||
|
|
|||
| CoprocessorStats { request_stats: CopRequestStatistics }, | |||
There was a problem hiding this comment.
Bad style to let raftstore import upper coprocessor mod.
nolouch
changed the title
src/coprocessor/endpoint.rs
Outdated
| pub type CopRequestStatistics = HashMap<u64, FlowStatistics>; | ||
|
|
||
| pub trait CopSender: Send + Clone { | ||
| fn send(&self, CopRequestStatistics) -> RaftStoreResult<()>; |
There was a problem hiding this comment.
Use your own Cop Result, don't use RaftStoreResult.
IMO, you should only know RaftStore in server mod
There was a problem hiding this comment.
umm, CopResult also needs use raftstore mod, for implement From from the RaftStoreResult. I'm going to extract the PD work in another pr.
There was a problem hiding this comment.
You don't need to support From, you can use box_try or if let Err(e) = + format! to return your own error.
|
LGTM But I suggest sending read statistics to the PD worker directly later. |
src/coprocessor/endpoint.rs
Outdated
|
|
||
| fn add_statistics_by_request(&mut self, id: u64, stats: &Statistics) { | ||
| let empty_stat = FlowStatistics::default(); | ||
| let flow_stats = self.request_stats.entry(id).or_insert(empty_stat); |
There was a problem hiding this comment.
s/empty_stat/FlowStatistics::default()
src/coprocessor/endpoint.rs
Outdated
| @@ -91,9 +117,16 @@ impl CopContext { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| fn add_statistics_by_request(&mut self, id: u64, stats: &Statistics) { | |||
There was a problem hiding this comment.
s/add_statistics_by_request/add_statistics_by_region
src/raftstore/store/store.rs
Outdated
|
|
||
| } | ||
|
|
||
| fn on_report_store_flow(&mut self) { |
There was a problem hiding this comment.
Too complicated to understand.
src/server/server.rs
Outdated
|
|
||
| impl<R: RaftStoreRouter + 'static> CopSender for CopReport<R> { | ||
| fn send(&self, stats: CopRequestStatistics) -> CopResult<()> { | ||
| box_try!(self.router.send(Msg::CoprocessorStats { |
There was a problem hiding this comment.
you can use try_send here
|
LGTM CI failed @nolouch |
…nto shuning/readbytes
…nto shuning/readbytes
report the read statistics.